Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Persist StateProof builders on disk #4553

Conversation

Aharonee
Copy link
Contributor

To support indefinite StateProofs recovery period (currently only 1000 intervals are supported), we must store the date required to build a StateProof transaction on disk.
This feature is a partial implementation, migrating the existing database and adding logic for persisting and restoring StateProof builders, as well as updating the unit tests accordingly.

Persist StateProof Builders

  • Add flag (used by Relay nodes) to enable this feature
  • Migrate the StateProof database
  • Insert every new builder into the database
  • Delete entry when the relevant StateProof transaction was confirmed
  • Load from DB (if flag enabled) on startup

Keep less builders in memory and send less signatures

This will be implemented in a separate PR

  • Replace the builders map with 2 (more?) builders: NextStateProofRoundBuilder and LatestRoundBuilder.

Testing

  • Enable persistBuilder flag for every test to make sure nothing breaks
  • Write new unit tests to verify the database state
  • Test: the number of builders in DB is consistent with the number of pending StateProofs
  • Test: if stateproof is not stalled, there should only be one builder stored in DB
  • Fix tests in worker_test.go to timeout when waiting on txmsg channel instead of blocking indefinitely
  • Update unit tests to make sure number of builders in memory is consistent with that in DB (will change in the next PR when limiting memory footprint)
  • Write e2e tests?

@codecov
Copy link

codecov bot commented Sep 15, 2022

Codecov Report

Merging #4553 (dda383d) into feature/stateproofs-recoverability (7de989a) will increase coverage by 0.09%.
The diff coverage is 78.88%.

@@                          Coverage Diff                           @@
##           feature/stateproofs-recoverability    #4553      +/-   ##
======================================================================
+ Coverage                               55.23%   55.33%   +0.09%     
======================================================================
  Files                                     398      398              
  Lines                                   50276    50389     +113     
======================================================================
+ Hits                                    27772    27881     +109     
+ Misses                                  20188    20185       -3     
- Partials                                 2316     2323       +7     
Impacted Files Coverage Δ
node/node.go 4.43% <ø> (ø)
stateproof/db.go 79.77% <74.07%> (-4.44%) ⬇️
stateproof/builder.go 80.14% <78.26%> (+3.22%) ⬆️
stateproof/signer.go 52.38% <80.00%> (+1.74%) ⬆️
crypto/stateproof/builder.go 88.88% <89.28%> (-0.64%) ⬇️
stateproof/worker.go 89.28% <100.00%> (-0.72%) ⬇️
network/wsNetwork.go 64.63% <0.00%> (-0.20%) ⬇️
ledger/acctupdates.go 69.89% <0.00%> (+0.59%) ⬆️
catchup/service.go 68.88% <0.00%> (+0.74%) ⬆️
... and 3 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@Aharonee Aharonee requested a review from id-ms September 19, 2022 06:18
@Aharonee Aharonee marked this pull request as ready for review September 19, 2022 06:53
stateproof/worker.go Outdated Show resolved Hide resolved
Copy link
Contributor

@id-ms id-ms left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good.
Have some suggestions to have a clearer code.

crypto/stateproof/builder.go Outdated Show resolved Hide resolved
crypto/stateproof/builder.go Outdated Show resolved Hide resolved
stateproof/builder.go Outdated Show resolved Hide resolved
stateproof/builder.go Outdated Show resolved Hide resolved
stateproof/builder.go Show resolved Hide resolved
stateproof/db.go Show resolved Hide resolved
stateproof/builder.go Outdated Show resolved Hide resolved
stateproof/signer.go Outdated Show resolved Hide resolved
stateproof/worker.go Outdated Show resolved Hide resolved
stateproof/worker.go Outdated Show resolved Hide resolved
stateproof/builder.go Outdated Show resolved Hide resolved
stateproof/db.go Outdated Show resolved Hide resolved
stateproof/builder.go Outdated Show resolved Hide resolved
stateproof/builder.go Outdated Show resolved Hide resolved
stateproof/builder.go Outdated Show resolved Hide resolved
stateproof/builder.go Outdated Show resolved Hide resolved
stateproof/worker.go Outdated Show resolved Hide resolved
stateproof/db_test.go Show resolved Hide resolved
stateproof/db_test.go Outdated Show resolved Hide resolved
stateproof/builder.go Outdated Show resolved Hide resolved
stateproof/builder.go Outdated Show resolved Hide resolved
@id-ms id-ms force-pushed the persist_stateproof_builders branch from bc366d2 to f2df8ef Compare October 20, 2022 13:30
stateproof/db.go Outdated Show resolved Hide resolved
@id-ms id-ms force-pushed the persist_stateproof_builders branch from 4179b32 to 9d78989 Compare October 21, 2022 13:47
@id-ms id-ms merged commit 598705d into algorand:feature/stateproofs-recoverability Oct 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants